Add write_load_forecast field to DataStreamsStats #92923

PhilKes · 2023-01-14T10:42:30Z

This PR Closes #91878 by adding the write_load_forecast field to the DataStreamsStats which is returned by GET /_data_streams/_stats

PhilKes · 2023-01-14T10:45:56Z

@dakrone could you please review this PR?

PhilKes · 2023-01-14T10:46:49Z

...eams/src/main/java/org/elasticsearch/datastreams/action/DataStreamsStatsTransportAction.java

-            return new DataStreamsStatsAction.DataStreamShardStats(indexShard.routingEntry(), storeStats, maxTimestamp);
+            var indexMetadata = indexService.getMetadata();
+            assert indexMetadata != null;
+            double writeLoadForecast = writeLoadForecaster.getForecastedWriteLoad(indexMetadata).orElse(0.0);


I was not entirely sure if this usage of the WriteLoadForecaster is correct, can somebody verify that?

The usage is ok except:

we should preserve empty rather than mapping it to 0. The difference between "write load is zero" and "write load is missing" is important.

there's no need to do this for each shard. Instead we should extract the write load from the index metadata in getResponseFactory.

we only want the write load from the write index of the data stream

Thanks @DaveCTurner for your feedback, I fixed the things you mentioned, writeLoadForecast is now Nullable and extracted only from the write index of the data stream

elasticsearchmachine · 2023-01-17T15:57:11Z

Pinging @elastic/es-data-management (Team:Data Management)

DaveCTurner

Thanks @PhilKes, I left a few more comments. Also I don't think this will even pass ./gradlew precommit as it stands, make sure you run that and fix up the problems it raises.

DaveCTurner · 2023-02-12T09:22:17Z

...eams/src/main/java/org/elasticsearch/datastreams/action/DataStreamsStatsTransportAction.java

+                    .index(indexAbstraction.getWriteIndex())
+                    .getForecastedWriteLoad();
+                if (writeLoadForeCast.isPresent()) {
+                    stats.writeLoadForecast = writeLoadForeCast.getAsDouble();


Looks better, thanks! However, this is now just the load for one shard of the write index. I think we want to multiply this by the number of shards in that index too.

Actually I can see value in knowing both the per-shard and per-index numbers.

Thanks, I added the multiplication with getNumberOfShards().
Do you think I should add the per-shard write load too? Having writeLoadForecast and writeLoadForecastPerShard?

Do you think I should add the per-shard write load too?

Yes please (let's make both of the names specific: ...PerShard and ...PerIndex)

Do you think I should add the per-shard write load too?

Yes please (let's make both of the names specific: ...PerShard and ...PerIndex)

Done

DaveCTurner · 2023-02-12T09:22:24Z

...eams/src/main/java/org/elasticsearch/datastreams/action/DataStreamsStatsTransportAction.java

            if (indexAbstraction.getType() == IndexAbstraction.Type.DATA_STREAM) {
                IndexAbstraction.DataStream dataStream = (IndexAbstraction.DataStream) indexAbstraction;
                AggregatedStats stats = aggregatedDataStreamsStats.computeIfAbsent(dataStream.getName(), s -> new AggregatedStats());
+                OptionalDouble writeLoadForeCast = clusterState.getMetadata()


nit:

Suggested change

OptionalDouble writeLoadForeCast = clusterState.getMetadata()

OptionalDouble writeLoadForecast = clusterState.getMetadata()

Fixed spelling error

DaveCTurner · 2023-02-12T09:23:32Z

...eams/src/main/java/org/elasticsearch/datastreams/action/DataStreamsStatsTransportAction.java

                AggregatedStats stats = aggregatedDataStreamsStats.computeIfAbsent(dataStream.getName(), s -> new AggregatedStats());
+                OptionalDouble writeLoadForeCast = clusterState.getMetadata()
+                    .index(indexAbstraction.getWriteIndex())
+                    .getForecastedWriteLoad();


Might this throw a NullPointerException sometimes?

Thats true, getWriteIndex() can return null, I added a null-check, and also precommit told me to use WriteLoadForecaster to get the writeload instead of IndexMetadata.getForecastedWriteLoad().

DaveCTurner

Implementation looks good to me. A few more non-implementation tasks needed:

I'd like there to be a test like the ones in DataStreamsStatsTests where they aren't null (for which I think you will need to make a test-specific plugin which overrides ClusterPlugin#createWriteLoadForecasters).
These new fields need mentioning in docs/reference/indices/data-stream-stats.asciidoc
Similarly it would be good to have some mention of them in modules/data-streams/src/yamlRestTest/resources/rest-api-spec/test/data_stream/120_data_streams_stats.yml, just to say that they really do exist in the output (null is fine here).

…reams_stats.yml

PhilKes · 2023-02-15T16:26:20Z

Implementation looks good to me. A few more non-implementation tasks needed:

* [ ]  I'd like there to be a test like the ones in `DataStreamsStatsTests` where they aren't `null` (for which I think you will need to make a test-specific plugin which overrides `ClusterPlugin#createWriteLoadForecasters`).

* [ ]  These new fields need mentioning in `docs/reference/indices/data-stream-stats.asciidoc`

* [ ]  Similarly it would be good to have some mention of them in `modules/data-streams/src/yamlRestTest/resources/rest-api-spec/test/data_stream/120_data_streams_stats.yml`, just to say that they really do exist in the output (null is fine here).

Thank you @DaveCTurner, but I need some guidance on this. I added the fields to the data-stream-stats.asciidoc, Im not quite sure when exactly the forecasted write load can be null, therefore I am not sure if a note on those fields is neccessary and what it should entail.

I added the fields to the 120_data_streams_stats.yml.

For the actual Test I tried to implement a new Test class DataStreamsStatsWriteLoadTests with the same setup as DataStreamsStatsTests, but with an additional new ClusterPlugin StaticWriteLoadForecasterPlugin that simply implements WriteLoadForecaster by returning 0.25 for any getForecastedWriteLoad call. I now wanted to implement a basic test like testStatsExistingDataStream, creating a random DataStream but including how many shards it should have (to actually test the perShard/perIndex values). But I am not sure how to control how many shards the DataStream/Write-Index is created with, can you help me on this?

EDIT: Ok I found out I can set the number of shards with the settings when creating the Template

new Template(Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, shards).build(),

I guess I will simply add a Test case to the DataStreamsStatsTests instead of creating a new Test class.

DaveCTurner · 2023-02-15T16:53:17Z

Im not quite sure when exactly the forecasted write load can be null, therefore I am not sure if a note on those fields is neccessary and what it should entail.

I'm ok not to mention this in the docs.

Ok I found out I can set the number of shards with the settings when creating the Template

Yes that sounds right.

…ssary notes in data-stream-stats.asciidoc

PhilKes · 2023-02-15T17:19:33Z

Im not quite sure when exactly the forecasted write load can be null, therefore I am not sure if a note on those fields is neccessary and what it should entail.

I'm ok not to mention this in the docs.

Ok I found out I can set the number of shards with the settings when creating the Template

Yes that sounds right.

Ok so I added the testWriteLoadStatsExistingDataStream to DataStreamsStatsTests with a datastream with 3 shards and asserting the per_shard and per_index forecast accordingly.

# Conflicts: # modules/data-streams/src/main/java/org/elasticsearch/datastreams/action/DataStreamsStatsTransportAction.java # modules/data-streams/src/test/java/org/elasticsearch/datastreams/DataStreamsStatsTests.java

Add write_load_forecast field to DataStreamsStats

3912c7c

elasticsearchmachine added v8.7.0 needs:triage Requires assignment of a team area label external-contributor Pull request authored by a developer outside the Elasticsearch team labels Jan 14, 2023

PhilKes commented Jan 14, 2023

View reviewed changes

pxsalehi added :Data Management/Data streams Data streams and their lifecycles and removed needs:triage Requires assignment of a team area label labels Jan 17, 2023

elasticsearchmachine added the Team:Data Management Meta label for data/management team label Jan 17, 2023

PhilKes mentioned this pull request Feb 4, 2023

write_load stats should be part of the _data_stream/stats #91878

Open

rjernst added v8.8.0 and removed v8.7.0 labels Feb 8, 2023

PhilKes added 6 commits February 11, 2023 17:29

Get optional write_load_forecast only for write index of data stream

45c1877

Add missing Nullable annoationt to writeLoadForecast fields

abefe58

Remove unnecessary indexMetadata in DataStreamsStatsTransportAction

1a70603

Fix DataStreamsStatsTests writeLoadForecast assertions

35dbab8

Remove WriteLoadForecaster from DataStreamsStatsTransportAction

bd49c4c

Fix minor formatting issues in DataStreamsStatsTransportAction

0ad66b5

DaveCTurner reviewed Feb 12, 2023

View reviewed changes

PhilKes added 3 commits February 13, 2023 18:11

Add null-check for writeIndex and use writeLoadForecaster

680e8ad

Fix minor formatting issues

dc987b6

Add writeLoadForecastPerShard and writeLoadForecastPerIndex

6e68185

DaveCTurner reviewed Feb 14, 2023

View reviewed changes

Add new write load fields to data-stream-stats.asciidoc + 120_data_st…

c33d67e

…reams_stats.yml

Add write_load_forecast Test to DataStreamsStatsTests + remove unnece…

b989b1d

…ssary notes in data-stream-stats.asciidoc

PhilKes requested a review from DaveCTurner March 15, 2023 11:28

gmarouli added v8.9.0 and removed v8.8.0 labels Apr 26, 2023

pugnascotia added v8.10.0 and removed v8.9.0 labels Jun 22, 2023

quux00 added v8.11.0 and removed v8.10.0 labels Aug 16, 2023

mattc58 added v8.12.0 and removed v8.11.0 labels Oct 4, 2023

brianseeders added v8.13.0 and removed v8.12.0 labels Dec 6, 2023

elasticsearchmachine added v8.14.0 and removed v8.13.0 labels Feb 14, 2024

elasticsearchmachine added v8.15.0 and removed v8.14.0 labels Apr 17, 2024

Merge branch 'main' into data-stream-write-load-stats

3b44190

# Conflicts: # modules/data-streams/src/main/java/org/elasticsearch/datastreams/action/DataStreamsStatsTransportAction.java # modules/data-streams/src/test/java/org/elasticsearch/datastreams/DataStreamsStatsTests.java

elasticsearchmachine added v8.16.0 and removed v8.15.0 labels Jul 4, 2024

mark-vieira added v9.0.0 and removed v8.16.0 labels Sep 11, 2024

elasticsearchmachine added v9.1.0 and removed v9.0.0 labels Jan 30, 2025

PhilKes closed this Apr 18, 2025

	OptionalDouble writeLoadForeCast = clusterState.getMetadata()
	OptionalDouble writeLoadForecast = clusterState.getMetadata()

Add write_load_forecast field to DataStreamsStats #92923

Add write_load_forecast field to DataStreamsStats #92923

Uh oh!

Conversation

PhilKes commented Jan 14, 2023

Uh oh!

PhilKes commented Jan 14, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DaveCTurner Feb 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elasticsearchmachine commented Jan 17, 2023

Uh oh!

DaveCTurner left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DaveCTurner left a comment

Choose a reason for hiding this comment

Uh oh!

PhilKes commented Feb 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DaveCTurner commented Feb 15, 2023

Uh oh!

PhilKes commented Feb 15, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

11 participants

DaveCTurner Feb 5, 2023 •

edited

Loading

PhilKes commented Feb 15, 2023 •

edited

Loading